-
-
Notifications
You must be signed in to change notification settings - Fork 12
merge dev to main (v3.0.0-beta.8) #280
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…on previous errors (#273) * fix(language): some references are not resolved; suppress validation on previous errors * update * addressing comments
* fix(policy): always wrap mutation in a transaction * fix error handling
Co-authored-by: ymc9 <[email protected]>
|
Warning Rate limit exceeded@ymc9 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 34 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughIntroduces a new policy operation "post-update" and replaces the policy helper function Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Claude finished @ymc9's task —— View job PR Review for v3.0.0-beta.8 ReleaseReviewing this PR that merges dev → main for the v3.0.0-beta.8 release with 831 additions and 373 deletions across 59 files. Todo List
🔍 Comprehensive Review Summary✅ Strengths & Well-Implemented Features1. Post-Update Policies - Major New Feature
2. Code Quality & Architecture
3. Language & Validation Improvements
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR merges development branch to main for the v3.0.0-beta.8 release, implementing support for post-update policies using before() function and transitioning away from the previous future() approach.
- Adds post-update policy functionality with
before()function to access entity values before updates - Replaces previous
future()implementation with newbefore()approach for accessing pre-update state - Updates version numbers across all packages to 3.0.0-beta.8
Reviewed Changes
Copilot reviewed 56 out of 59 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/runtime/src/plugins/policy/policy-handler.ts | Implements core post-update policy logic with before-update entity loading and validation |
| packages/language/res/stdlib.zmodel | Updates function name from future() to before() with corrected documentation |
| packages/runtime/test/policy/crud/post-update.test.ts | Comprehensive test suite for new post-update policy functionality |
| tests/regression/test/issue-274/regression.test.ts | New regression test for issue 274 using updated test infrastructure |
| packages/testtools/src/project.ts | Enhanced test project creation to accept zmodel content parameter |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/language/res/stdlib.zmodel (1)
669-711: Document the newpost-updateoperation in the attribute docs.
'post-update'is now part of the completion hints, but the parameter docs above@@allow/@@denystill say the list is just “create/read/update/delete/all”. Please update the text so the documentation matches the accepted operations and avoids confusing users.packages/sdk/src/prisma/prisma-schema-generator.ts (1)
262-273: Add test coverage for@default(auth())invocations
No existing test schemas include a@default(auth(...))attribute. Add both simple and nestedauth()cases inpackages/runtime/test/schemas(andpackages/language/test) to ensureisDefaultWithAuthInvocationreliably detects them.
🧹 Nitpick comments (2)
TODO.md (1)
104-105: Fix Markdown list indentationThese new sub-items use four leading spaces, but our markdownlint config (MD007) expects a two-space indent for nested bullets. Please drop the indent to two spaces so the checklist passes lint.
packages/runtime/test/policy/migrated/petstore-sample.test.ts (1)
5-5: Consider adding test cases for post-update constraint validation.The test suite has been re-enabled to work with the new
before()/post-updatesemantics. However, the current test only validates basic CRUD operations and doesn't verify the post-update constraints defined in the schema (e.g., preventing updates toname/categoryfields, or preventingorderIdfrom being changed once set).Consider adding test cases such as:
- Attempting to update
nameorcategoryon an existing pet (should fail post-update validation)- Attempting to change
orderIdafter it's been set (should fail post-update validation)- Verifying that
orderIdCAN be set from null (should succeed)Would you like me to generate these additional test cases?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (56)
TODO.md(1 hunks)package.json(1 hunks)packages/cli/package.json(1 hunks)packages/common-helpers/package.json(1 hunks)packages/create-zenstack/package.json(1 hunks)packages/dialects/sql.js/package.json(1 hunks)packages/eslint-config/package.json(1 hunks)packages/ide/vscode/package.json(1 hunks)packages/language/package.json(1 hunks)packages/language/res/stdlib.zmodel(3 hunks)packages/language/src/index.ts(1 hunks)packages/language/src/utils.ts(1 hunks)packages/language/src/validators/attribute-application-validator.ts(3 hunks)packages/language/src/validators/expression-validator.ts(2 hunks)packages/language/src/zmodel-document-builder.ts(1 hunks)packages/language/src/zmodel-linker.ts(8 hunks)packages/language/src/zmodel-scope.ts(2 hunks)packages/language/test/attribute-application.test.ts(1 hunks)packages/runtime/package.json(1 hunks)packages/runtime/src/client/contract.ts(1 hunks)packages/runtime/src/client/crud/operations/base.ts(1 hunks)packages/runtime/src/client/errors.ts(3 hunks)packages/runtime/src/client/executor/zenstack-query-executor.ts(4 hunks)packages/runtime/src/client/options.ts(2 hunks)packages/runtime/src/plugins/policy/errors.ts(2 hunks)packages/runtime/src/plugins/policy/expression-transformer.ts(4 hunks)packages/runtime/src/plugins/policy/plugin.ts(1 hunks)packages/runtime/src/plugins/policy/policy-handler.ts(14 hunks)packages/runtime/src/plugins/policy/types.ts(2 hunks)packages/runtime/src/plugins/policy/utils.ts(2 hunks)packages/runtime/src/utils/expression-utils.ts(1 hunks)packages/runtime/test/policy/crud/post-update.test.ts(1 hunks)packages/runtime/test/policy/migrated/multi-field-unique.test.ts(1 hunks)packages/runtime/test/policy/migrated/multi-id-fields.test.ts(6 hunks)packages/runtime/test/policy/migrated/nested-to-one.test.ts(2 hunks)packages/runtime/test/policy/migrated/petstore-sample.test.ts(1 hunks)packages/runtime/test/policy/migrated/todo-sample.test.ts(1 hunks)packages/runtime/test/policy/migrated/toplevel-operations.test.ts(3 hunks)packages/runtime/test/policy/todo-sample.test.ts(1 hunks)packages/runtime/test/schemas/petstore/schema.ts(1 hunks)packages/runtime/test/schemas/petstore/schema.zmodel(1 hunks)packages/runtime/test/schemas/todo/schema.ts(2 hunks)packages/runtime/test/schemas/todo/todo.zmodel(2 hunks)packages/sdk/package.json(1 hunks)packages/sdk/src/prisma/prisma-schema-generator.ts(4 hunks)packages/tanstack-query/package.json(1 hunks)packages/testtools/package.json(1 hunks)packages/testtools/src/project.ts(2 hunks)packages/typescript-config/package.json(1 hunks)packages/vitest-config/package.json(1 hunks)packages/zod/package.json(1 hunks)samples/blog/package.json(1 hunks)tests/e2e/package.json(1 hunks)tests/regression/package.json(1 hunks)tests/regression/test/issue-204/regression.test.ts(1 hunks)tests/regression/test/issue-274/regression.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
{packages,samples,tests}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place packages only under
packages/,samples/, ortests/
Files:
packages/runtime/package.jsonpackages/language/package.jsonpackages/runtime/src/plugins/policy/types.tspackages/language/src/utils.tspackages/language/src/zmodel-scope.tspackages/runtime/test/policy/migrated/todo-sample.test.tspackages/cli/package.jsonpackages/runtime/src/plugins/policy/errors.tspackages/runtime/src/plugins/policy/plugin.tspackages/runtime/src/client/crud/operations/base.tspackages/runtime/src/client/options.tspackages/testtools/src/project.tspackages/language/src/index.tspackages/typescript-config/package.jsontests/regression/package.jsonpackages/runtime/src/client/errors.tspackages/runtime/src/plugins/policy/utils.tspackages/runtime/test/policy/migrated/petstore-sample.test.tspackages/sdk/src/prisma/prisma-schema-generator.tspackages/sdk/package.jsonpackages/runtime/src/client/contract.tspackages/runtime/test/schemas/todo/schema.tspackages/common-helpers/package.jsontests/regression/test/issue-204/regression.test.tstests/regression/test/issue-274/regression.test.tspackages/runtime/test/schemas/petstore/schema.zmodelpackages/ide/vscode/package.jsonpackages/create-zenstack/package.jsonpackages/runtime/test/policy/migrated/nested-to-one.test.tssamples/blog/package.jsonpackages/language/src/validators/expression-validator.tspackages/runtime/test/policy/crud/post-update.test.tspackages/vitest-config/package.jsonpackages/language/src/validators/attribute-application-validator.tspackages/runtime/src/plugins/policy/expression-transformer.tspackages/runtime/test/policy/migrated/toplevel-operations.test.tspackages/runtime/src/client/executor/zenstack-query-executor.tspackages/zod/package.jsonpackages/language/test/attribute-application.test.tspackages/language/res/stdlib.zmodelpackages/language/src/zmodel-linker.tspackages/tanstack-query/package.jsonpackages/dialects/sql.js/package.jsontests/e2e/package.jsonpackages/runtime/test/policy/migrated/multi-field-unique.test.tspackages/runtime/test/schemas/petstore/schema.tspackages/eslint-config/package.jsonpackages/language/src/zmodel-document-builder.tspackages/testtools/package.jsonpackages/runtime/test/policy/todo-sample.test.tspackages/runtime/src/utils/expression-utils.tspackages/runtime/test/schemas/todo/todo.zmodelpackages/runtime/test/policy/migrated/multi-id-fields.test.tspackages/runtime/src/plugins/policy/policy-handler.ts
package.json
📄 CodeRabbit inference engine (CLAUDE.md)
Pin the repository package manager to
[email protected]via thepackageManagerfield
Files:
package.json
**/schema.ts
📄 CodeRabbit inference engine (CLAUDE.md)
The generated TypeScript schema should be named
schema.ts
Files:
packages/runtime/test/schemas/todo/schema.tspackages/runtime/test/schemas/petstore/schema.ts
**/schema.zmodel
📄 CodeRabbit inference engine (CLAUDE.md)
Name ZModel schema files
schema.zmodel
Files:
packages/runtime/test/schemas/petstore/schema.zmodel
packages/ide/vscode/package.json
📄 CodeRabbit inference engine (CLAUDE.md)
The
ide/vscodepackage is intentionally versioned differently from other packages
Files:
packages/ide/vscode/package.json
tests/e2e/**
📄 CodeRabbit inference engine (CLAUDE.md)
End-to-end tests must live under
tests/e2e/
Files:
tests/e2e/package.json
🧠 Learnings (1)
📚 Learning: 2025-09-04T12:38:14.150Z
Learnt from: CR
PR: zenstackhq/zenstack-v3#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-04T12:38:14.150Z
Learning: Applies to packages/ide/vscode/package.json : The `ide/vscode` package is intentionally versioned differently from other packages
Applied to files:
packages/ide/vscode/package.json
🧬 Code graph analysis (22)
packages/runtime/src/plugins/policy/types.ts (1)
packages/runtime/src/client/contract.ts (2)
CRUD_EXT(219-219)CRUD_EXT(229-229)
packages/language/src/utils.ts (4)
packages/runtime/src/plugins/policy/utils.ts (1)
isBeforeInvocation(158-160)packages/language/src/ast.ts (2)
AstNode(4-4)AstNode(66-71)packages/language/src/generated/ast.ts (1)
isInvocationExpr(542-544)packages/sdk/src/model-utils.ts (1)
isFromStdlib(85-88)
packages/language/src/zmodel-scope.ts (2)
packages/language/src/utils.ts (1)
isBeforeInvocation(449-451)packages/runtime/src/plugins/policy/utils.ts (1)
isBeforeInvocation(158-160)
packages/runtime/src/plugins/policy/errors.ts (1)
packages/runtime/src/client/errors.ts (1)
ZenStackError(4-4)
packages/runtime/src/plugins/policy/plugin.ts (2)
packages/runtime/src/client/plugin.ts (1)
OnKyselyQueryArgs(188-193)packages/runtime/src/plugins/policy/policy-handler.ts (1)
PolicyHandler(62-990)
packages/runtime/src/client/crud/operations/base.ts (2)
packages/runtime/src/client/executor/zenstack-query-executor.ts (1)
kysely(60-62)packages/runtime/src/plugins/policy/policy-handler.ts (1)
kysely(70-72)
packages/runtime/src/client/options.ts (1)
packages/runtime/src/client/contract.ts (2)
CRUD_EXT(219-219)CRUD_EXT(229-229)
packages/runtime/src/plugins/policy/utils.ts (2)
packages/language/src/utils.ts (1)
isBeforeInvocation(449-451)packages/runtime/src/schema/expression.ts (1)
ExpressionUtils(16-120)
packages/sdk/src/prisma/prisma-schema-generator.ts (3)
packages/language/src/generated/ast.ts (2)
DataFieldAttribute(348-353)DataFieldAttribute(355-355)packages/language/src/utils.ts (1)
isAuthInvocation(64-66)packages/sdk/src/ts-schema-generator.ts (1)
isAuthInvocation(651-655)
packages/runtime/test/schemas/todo/schema.ts (1)
packages/runtime/src/schema/expression.ts (1)
ExpressionUtils(16-120)
tests/regression/test/issue-204/regression.test.ts (1)
tests/regression/test/issue-204/models.ts (3)
Configuration(11-11)ShirtColor(12-12)ShirtColor(13-13)
tests/regression/test/issue-274/regression.test.ts (2)
packages/testtools/src/project.ts (1)
createTestProject(5-71)packages/cli/src/utils/exec-utils.ts (1)
execSync(6-15)
packages/language/src/validators/expression-validator.ts (2)
packages/language/src/generated/ast.ts (3)
MemberAccessExpr(546-551)MemberAccessExpr(553-553)isDataModel(391-393)packages/language/src/utils.ts (1)
isBeforeInvocation(449-451)
packages/runtime/test/policy/crud/post-update.test.ts (1)
packages/runtime/test/policy/utils.ts (1)
createPolicyTestClient(14-25)
packages/language/src/validators/attribute-application-validator.ts (1)
packages/language/src/utils.ts (1)
isBeforeInvocation(449-451)
packages/runtime/src/plugins/policy/expression-transformer.ts (5)
packages/sdk/src/schema/schema.ts (2)
SchemaDef(10-18)GetModels(109-109)packages/runtime/src/client/contract.ts (2)
CRUD_EXT(219-219)CRUD_EXT(229-229)packages/sdk/src/schema/expression.ts (1)
MemberExpression(27-31)packages/runtime/src/plugins/policy/utils.ts (1)
isBeforeInvocation(158-160)packages/common-helpers/src/tiny-invariant.ts (1)
invariant(4-14)
packages/runtime/src/client/executor/zenstack-query-executor.ts (2)
packages/runtime/src/client/executor/zenstack-driver.ts (2)
compiledQuery(181-188)connection(129-170)packages/runtime/src/client/errors.ts (2)
ZenStackError(4-4)QueryError(18-22)
packages/language/test/attribute-application.test.ts (1)
packages/language/test/utils.ts (1)
loadSchemaWithError(21-39)
packages/language/src/zmodel-linker.ts (1)
packages/language/src/utils.ts (1)
isBeforeInvocation(449-451)
packages/runtime/test/schemas/petstore/schema.ts (1)
packages/runtime/src/schema/expression.ts (1)
ExpressionUtils(16-120)
packages/runtime/src/utils/expression-utils.ts (1)
packages/sdk/src/schema/expression.ts (9)
LiteralExpression(12-15)ArrayExpression(17-20)FieldExpression(22-25)MemberExpression(27-31)BinaryExpression(39-44)UnaryExpression(33-37)CallExpression(46-50)ThisExpression(52-54)NullExpression(56-58)
packages/runtime/src/plugins/policy/policy-handler.ts (7)
packages/runtime/src/client/plugin.ts (1)
ProceedKyselyQueryFunction(195-195)packages/runtime/src/client/query-utils.ts (3)
requireField(38-56)requireIdFields(63-70)requireModel(25-31)packages/runtime/src/plugins/policy/utils.ts (6)
conjunction(49-67)isBeforeInvocation(158-160)trueNode(21-23)falseNode(28-30)disjunction(69-87)buildIsFalse(127-139)packages/runtime/src/utils/expression-utils.ts (1)
ExpressionVisitor(15-58)packages/sdk/src/schema/expression.ts (1)
MemberExpression(27-31)packages/common-helpers/src/tiny-invariant.ts (1)
invariant(4-14)packages/runtime/src/client/contract.ts (2)
CRUD_EXT(219-219)CRUD_EXT(229-229)
🪛 markdownlint-cli2 (0.18.1)
TODO.md
104-104: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
105-105: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-test (20.x, sqlite)
- GitHub Check: build-test (20.x, postgresql)
- GitHub Check: claude-review
🔇 Additional comments (50)
tests/regression/test/issue-204/regression.test.ts (1)
1-9: LGTM! Clean simplification of test structure.Removing the
describewrapper for a single test case is appropriate and reduces unnecessary nesting. The test effectively serves as a regression check for issue 204, verifying that theConfigurationtype works correctly with optionalteamColorsand that the code compiles and runs without errors.packages/testtools/package.json (1)
3-3: LGTM! Version bump aligned with beta.8 release.The version increment from 3.0.0-beta.7 to 3.0.0-beta.8 is consistent with the PR objective and the coordinated monorepo-wide version bump.
packages/zod/package.json (1)
3-3: LGTM! Version bump aligned with beta.8 release.The version increment from 3.0.0-beta.7 to 3.0.0-beta.8 is consistent with the PR objective and the coordinated monorepo-wide version bump.
packages/sdk/package.json (1)
3-3: LGTM! Version bump aligned with beta.8 release.The version increment from 3.0.0-beta.7 to 3.0.0-beta.8 is consistent with the PR objective and the coordinated monorepo-wide version bump.
packages/language/package.json (1)
4-4: LGTM! Version bump aligned with beta.8 release.The version increment from 3.0.0-beta.7 to 3.0.0-beta.8 is consistent with the PR objective and the coordinated monorepo-wide version bump. This version bump correctly accompanies the language changes for before()/post-update semantics mentioned in the AI summary.
packages/dialects/sql.js/package.json (1)
3-3: LGTM! Version bump aligned with beta.8 release.The version increment from 3.0.0-beta.7 to 3.0.0-beta.8 is consistent with the PR objective and the coordinated monorepo-wide version bump.
packages/runtime/test/policy/migrated/todo-sample.test.ts (1)
373-430: Post-update checks back under coverageRe-enabling this suite restores the regression guardrails for the policy handoff; the assertions still line up with the shared fixtures.
packages/ide/vscode/package.json (1)
4-4: VSCode extension version bump confirmedKeeping this package on its distinct 3.0.9 track honors the prescribed divergence. As per coding guidelines
packages/cli/package.json (1)
6-6: CLI version aligned with beta.8 releaseThe isolated bump to 3.0.0-beta.8 looks good for the monorepo release.
packages/vitest-config/package.json (1)
4-4: Vitest config version bump looks goodNothing else changed; ready for the beta.8 publish.
package.json (1)
3-3: Root package version synchronized3.0.0-beta.8 matches the coordinated release numbering.
packages/runtime/test/policy/todo-sample.test.ts (1)
386-444: Thanks for re-enabling this coverage. The scenario now exercises the new post-update policy checks end-to-end, so having it active will prevent regressions around owner reassignment.tests/regression/package.json (1)
3-3: Version bump looks good. Matches the repo-wide 3.0.0-beta.8 release alignment.packages/runtime/package.json (1)
3-3: Confirming runtime version sync. Keeping @zenstackhq/runtime at 3.0.0-beta.8 stays consistent with the coordinated release.samples/blog/package.json (1)
3-3: Sample app version updated appropriately. This aligns the blog sample with the beta.8 release train.packages/create-zenstack/package.json (1)
3-3: CLI scaffold version matches the release. No other changes needed for beta.8.packages/tanstack-query/package.json (1)
3-3: Version bump aligns with beta.8 releaseMatches the coordinated monorepo version lift; nothing else required here.
packages/runtime/test/policy/migrated/nested-to-one.test.ts (1)
200-255: Activated test confirms post-update guardRe-enabling the scenario alongside the new
@@allow('post-update', value > 2)keeps the legacy behavior (pre-state must exceed 1, post-state must exceed 2) and gives us coverage for the new hook—looks solid.packages/runtime/test/schemas/petstore/schema.ts (1)
95-97: Generated post-update rule mirrors new policy semanticsThe generated expression correctly encodes the new
@@allow('update')plus@@allow('post-update')logic withbefore()accessors, so the runtime sees the same constraints our tests expect.packages/runtime/test/schemas/petstore/schema.zmodel (1)
38-41: Verify the interaction between general update permission and post-update constraint.The new policy structure uses two rules: Line 38 permits any authenticated user to update, while Line 41 enforces post-update constraints. This means update attempts are allowed upfront but will fail post-update if
nameorcategorychanged, or iforderIdwas already set.This is a significant semantic shift from the previous approach. Ensure that:
- The runtime correctly rolls back transactions when post-update rules fail
- Error messages clearly indicate which fields can be updated
- The comment on Line 40 accurately reflects that only
orderIdtransitions from null are permitted (name/category must remain unchanged)Based on learnings about the new
before()andpost-updatesemantics introduced in this PR.packages/sdk/src/prisma/prisma-schema-generator.ts (2)
33-33: LGTM: Import updated to support auth invocation detection.The import correctly adds
isAuthInvocationutility to support the refactored default attribute filtering logic.
244-245: LGTM: Comment and filter logic updated for auth-based invocations.The comment and filter predicate have been correctly updated to reflect that
@defaultattributes usingauth()are now handled outside Prisma, replacing the previous plugin-invocation pattern.packages/runtime/test/schemas/todo/schema.ts (1)
1-4: Generated file - changes look correct.This is an auto-generated schema file. The new post-update deny policies have been correctly generated using the
before()function and proper ExpressionUtils API calls. The policies enforce thatownerIdcannot be changed during updates.packages/runtime/src/utils/expression-utils.ts (1)
15-58: LGTM! Well-designed visitor pattern implementation.The
ExpressionVisitorclass provides a clean, extensible visitor pattern for traversing expression ASTs. The implementation correctly:
- Uses exhaustive pattern matching via ts-pattern
- Provides protected methods for subclass customization
- Recursively visits all child expressions
- Handles optional fields appropriately (e.g.,
argsin CallExpression)packages/language/src/index.ts (1)
72-78: LGTM! Improved validation configuration.The updated validation configuration provides explicit, fine-grained control over when document building should stop on errors. Setting all three flags to
truemaintains fail-fast behavior while being more self-documenting than a simple boolean.packages/testtools/src/project.ts (2)
5-5: LGTM! Clean API extension.The optional
zmodelContentparameter maintains backward compatibility while enabling test scenarios to inject custom schema definitions.
66-68: LGTM! Proper conditional file writing.The conditional block correctly writes the provided zmodel content to
schema.zmodelwhen supplied, following the expected naming convention.packages/language/src/zmodel-scope.ts (1)
40-40: LGTM! Consistent rename from future() to before().The import, usage, and comment have been consistently updated to reflect the rename from
isFutureInvocationtoisBeforeInvocation. The scope resolution logic correctly resolvesbefore()invocations to the containing model.Also applies to: 173-176
packages/runtime/src/plugins/policy/types.ts (1)
1-12: CRUD_EXT integration looks soundPolicyOperation now reuses the extended CRUD set from the client contract, aligning policy typing with the new 'post-update' operation. Nice and tidy.
tests/regression/test/issue-274/regression.test.ts (1)
5-27: Regression scenario wired end-to-endThe temp-project scaffold plus CLI migrate run reproduces issue 274 realistically, giving us solid coverage on the reported workflow.
packages/runtime/src/plugins/policy/utils.ts (1)
16-160: Helper covers runtime before() detectionAdding isBeforeInvocation centralizes the call-expression check for before(), keeping runtime policy logic aligned with the language validators.
packages/runtime/src/plugins/policy/errors.ts (1)
1-33: Error now fits the ZenStackError hierarchyExtending ZenStackError keeps policy rejection errors aligned with the shared error stack while preserving the customizable message.
packages/runtime/src/client/contract.ts (1)
217-229: Contract exposes extended CRUD literal setPublishing CRUD_EXT as both a type and tuple constant makes it straightforward for callers to consume the new 'post-update' operation.
packages/language/src/validators/attribute-application-validator.ts (1)
169-182: Good guard forbefore()usageRestricting
before()to pure"post-update"rules prevents models from sneaking pre-update reads into other operations, which keeps policy semantics consistent. Nice catch.packages/language/src/validators/expression-validator.ts (1)
73-77: Great validation for relation access inbefore()Blocking
before().relationat the expression layer closes an easy loophole for reaching fresh relation snapshots. This error path is precise and keeps diagnostics user-friendly.packages/runtime/src/plugins/policy/plugin.ts (1)
25-28: Signature alignment looks goodUpdating the handler call to the new
(query, proceed)contract keeps the plugin wired correctly with the latest policy handler API.packages/runtime/test/schemas/todo/todo.zmodel (2)
120-122: LGTM! Post-update policy correctly prevents owner changes.The new
@@deny('post-update', before().ownerId != ownerId)rule properly uses thebefore()helper to compare the previous owner with the new owner, denying updates that attempt to transfer ownership.
149-150: LGTM! Consistent post-update policy implementation.The
@@deny('post-update', before().ownerId != ownerId)rule matches the pattern established in the List model, ensuring consistent enforcement of ownership immutability across the schema.packages/language/src/zmodel-document-builder.ts (1)
4-25: LGTM! Constructor-based validation configuration is cleaner.The new approach correctly configures validation to stop after lexing, parsing, and linking errors at construction time. The conditional logic properly handles both boolean and object validation options.
packages/runtime/src/client/errors.ts (2)
1-4: LGTM! Proper error hierarchy base class.The new
ZenStackErrorbase class correctly extendsErrorand provides a common ancestor for all ZenStack runtime errors, enabling centralized error handling.
9-36: LGTM! Error classes correctly extend the new base.All four error classes now properly extend
ZenStackErrorwhile maintaining their existing constructor signatures, ensuring backward compatibility.packages/language/src/zmodel-linker.ts (5)
1-15: LGTM! Import updates align with the before() rename.The addition of
ReferenceInfoand replacement ofisFutureExprwithisBeforeInvocationcorrectly support the refactored linking architecture and the renamed policy helper.Also applies to: 54-63
97-109: LGTM! Refactored linking logic with proper early exits.The new
linkReferencesignature usingReferenceInfoenables cleaner separation of concerns, with early exit for already-linked references and prioritized scope provider resolution before default linking.
276-301: LGTM! Invocation resolution updated for before() semantics.The updated
linkReferencecall and the replacement ofisFutureExprwithisBeforeInvocationcorrectly handle the renamed helper function, resolvingbefore()to the containing data model.
115-133: LGTM! Scope provider resolution properly stores results.The updated
resolveFromScopeProviderscorrectly accepts aDefaultReference, stores the resolution result, creates the appropriate description, and updates the document's reference array.
495-502: LGTM! Cleaner reference iteration with streamReferences.Replacing manual property iteration with
AstUtils.streamReferencesis more idiomatic and maintainable while preserving the same linking behavior.packages/runtime/src/plugins/policy/expression-transformer.ts (4)
23-23: LGTM! Type updates support post-update operations.The import and type changes from
CRUDtoCRUD_EXTcorrectly support the newpost-updateoperation, and theisBeforeInvocationimport enables the newbefore()handling logic.Also applies to: 53-53, 58-58
440-443: LGTM! Consistent use of class property.Using
this.authinstead of extracting from context is more consistent with the class design and leverages the existing getter.
460-467: LGTM! Correct before() member access implementation.The new
before()handling properly:
- Validates usage in
post-updatecontext only- Restricts access to scalar fields (single-level member access)
- Returns a table-qualified reference to the
$beforejoin table created by the policy handlerThe invariants provide clear error messages for misuse.
455-458: LGTM! Comment clarity improvement.Adding parentheses to "
auth()member access" improves readability and consistency with the newbefore()comment.
|
Claude finished @ymc9's task —— View job PR Review for v3.0.0-beta.8 ReleaseReviewing this PR that merges dev → main for the v3.0.0-beta.8 release with 831 additions and 373 deletions across 59 files. Todo List
🔍 Comprehensive Review Summary✅ Strengths & Well-Implemented Features1. Post-Update Policies - Major New Feature
2. Code Quality & Architecture
3. Language & Validation Improvements
|
Summary by CodeRabbit
New Features
Documentation
Tests
Chores